Skip to content

Conversation

@capelski
Copy link

@capelski capelski commented May 2, 2021

Platforms affected

Google Chrome on Windows

What does this PR do?

Fix propTypes issues on browsers

What testing has been done on this change?

Launched the app in the Expo Client

@capelski
Copy link
Author

capelski commented May 2, 2021

If somebody is willing to provide a better solution, flow types should be used instead

@Gakusco
Copy link

Gakusco commented May 2, 2021

Hello, why not?

containerStyle: ViewPropTypes ? ViewPropTypes.style : null,

@capelski
Copy link
Author

capelski commented May 3, 2021

Hello, why not?

containerStyle: ViewPropTypes ? ViewPropTypes.style : null,

React expects propTypes to be functions:

index.js:1 Warning: Failed prop type: Carousel: prop type `slideStyle` is invalid; it must be a function, usually from the `prop-types` package, but received `object`.

image

Using null instead of a null-returning function would raise warnings in development mode

Copy link

@rabidkitten rabidkitten left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I was having the same issue as described when running a React Native app on the web. It was working fine on my phone (iOS), but not in a web browser. As a comment, I tried the changes outlined herein and was able to use the component on the web as well as on the phone. It appears that these changes fix the issue.

@NiclasHorstad-acn
Copy link

Hoping this PR can be merged soon. The workarounds posted elsewhere on Github using custom postinstall scripts to insert ViewPropTypes back into the package hurts my eyes. I know web isn't officially supported but it would be great nonetheless.

@the-simian
Copy link

Merging this would be very much appreciated. I'd like to avoid any of the post install hacks. being able to run the same codebase on every mobile platform and also the web is incredibly useful, especially in the development stage.

@acidjunk
Copy link

I was having the same issue as described when running a React Native app on the web. With the changes the slider loads OK; but sliding (in de web variant) only works with the keyboard; not with the mouse. Am I doing something wrong?

@capelski
Copy link
Author

I was having the same issue as described when running a React Native app on the web. With the changes the slider loads OK; but sliding (in de web variant) only works with the keyboard; not with the mouse. Am I doing something wrong?

@acidjunk Unfortunately this library doesn't explicitly support web browsers; the behavior you describe is actually the expected behavior. You will only be able to slide the carrousel in web browsers when using tactile screens or when using the browser device emulator.

P.D. This PR is not adding web browser compatibility to the library but just fixing a null pointer error that causes a critical crash in web browsers

@aliburhankeskin
Copy link

Hello, I started getting this error in Expo SDK version 51. When will this PR be merged?

@capelski
Copy link
Author

capelski commented May 9, 2024

Hello, I started getting this error in Expo SDK version 51. When will this PR be merged?

The PR has been open for 3 years now. I'd say this repository is not maintained unfortunately.

@aliburhankeskin
Copy link

Hello, I started getting this error in Expo SDK version 51. When will this PR be merged?

The PR has been open for 3 years now. I'd say this repository is not maintained unfortunately.

thanks for answer . By the way, this PR has also been improved. PR has been waiting for 2 weeks. #1015

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

7 participants